Fix HostDBInfo::is_down condition #12846
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a logical error in the HostDBInfo::is_down method where the condition was inverted, causing it to incorrectly determine when a host should be considered down based on its failure window.
Changes:
- Corrected the comparison operator in
is_down()from<to>=to properly check if a host is still within its failure window
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bryancall
left a comment
bryancall
left a comment
There was a problem hiding this comment.
Please make comments, it is confusing
|
Approved. Maybe check with @serrislew and @cmcfarlen as well since it sounds like they looked into this previously. |
|
I wrote comment to clarify. Please take another look. |
|
I talked with @serrislew and @cmcfarlen, we found her patch has almost the same change in this function with other changes. And we agreed with shipping this PR first. |
bryancall
left a comment
bryancall
left a comment
There was a problem hiding this comment.
LGTM. The inverted condition is clearly a bug — last_fail + fail_window < now returns true when the fail window has expired (host should be UP), not when the host is still down. The fix correctly flips this to now <= last_fail + fail_window.
The ASCII timeline diagram in the comment is a nice addition — makes the time-window semantics immediately clear.
SRV-only impact, CI green, straightforward fix.
3 participants
The condition of
HostDBInfo::is_downseems opposite.This function is only used by
HostDBRecord::select_best_srv(), so only SRV record case is affected.trafficserver/src/iocore/hostdb/HostDB.cc
Lines 1609 to 1611 in 2a30bcc